babl: use snprintf instead of sprintf
authorTobias Stoeckmann <tobias@stoeckmann.org>
Tue, 24 Oct 2017 18:02:15 +0000 (20:02 +0200)
committerØyvind Kolås <pippin@gimp.org>
Wed, 25 Oct 2017 14:48:41 +0000 (16:48 +0200)
Using sprintf with environment variables is dangerous, because it can
easily lead to out of boundary writes on heap space.

While at it, replace sprintf calls with snprintf where proper
boundary checks are possible and required.

Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
babl/babl-cache.c
babl/babl-conversion.c
babl/babl-format.c
babl/babl-icc.c
babl/babl-internal.c
babl/babl-memory.c
babl/babl-palette.c
babl/babl-space.c
babl/babl-trc.c

index 4954e4ba983de8a5e73d7c89fdb4f90281e86f9c..13badc58ef19e51bc612189fcef326893e2614da 100644 (file)
@@ -82,16 +82,16 @@ static const char *fish_cache_path (void)
   path[sizeof (path) - 1] = '\0';
 #ifndef _WIN32
   if (getenv ("XDG_CACHE_HOME"))
-    sprintf (path, "%s/babl/babl-fishes", getenv("XDG_CACHE_HOME"));
+    snprintf (path, sizeof (path), "%s/babl/babl-fishes", getenv("XDG_CACHE_HOME"));
   else if (getenv ("HOME"))
-    sprintf (path, "%s/.cache/babl/babl-fishes", getenv("HOME"));
+    snprintf (path, sizeof (path), "%s/.cache/babl/babl-fishes", getenv("HOME"));
 #else
 {
   char win32path[4096];
   if (SHGetFolderPathA (NULL, CSIDL_LOCAL_APPDATA, NULL, SHGFP_TYPE_CURRENT, win32path) == S_OK)
-    sprintf (path, "%s\\%s\\babl-fishes.txt", win32path, BABL_LIBRARY);
+    snprintf (path, sizeof (path), "%s\\%s\\babl-fishes.txt", win32path, BABL_LIBRARY);
   else if (getenv ("TEMP"))
-    sprintf (path, "%s\\babl-fishes.txt", getenv("TEMP"));
+    snprintf (path, sizeof (path), "%s\\babl-fishes.txt", getenv("TEMP"));
 }
 #endif
 
@@ -153,13 +153,13 @@ static const char *cache_header (void)
 {
   static char buf[2048];
   if (strchr (BABL_GIT_VERSION, ' ')) // we must be building from tarball
-    sprintf (buf, "#%i.%i.%i BABL_PATH_LENGTH=%d BABL_TOLERANCE=%f",
+    snprintf (buf, sizeof (buf),
+             "#%i.%i.%i BABL_PATH_LENGTH=%d BABL_TOLERANCE=%f",
              BABL_MAJOR_VERSION, BABL_MINOR_VERSION, BABL_MICRO_VERSION,
              _babl_max_path_len (), _babl_legal_error ());
   else
-    sprintf (buf, "#%s BABL_PATH_LENGTH=%d BABL_TOLERANCE=%f",
-             BABL_GIT_VERSION,
-             _babl_max_path_len (), _babl_legal_error ());
+    snprintf (buf, sizeof (buf), "#%s BABL_PATH_LENGTH=%d BABL_TOLERANCE=%f",
+             BABL_GIT_VERSION, _babl_max_path_len (), _babl_legal_error ());
   return buf;
 }
 
@@ -170,7 +170,7 @@ void babl_store_db (void)
   char *tmpp = calloc(8000,1);
   FILE *dbfile;
 
-  sprintf (tmpp, "%s~", fish_cache_path ());
+  snprintf (tmpp, 8000, "%s~", fish_cache_path ());
   dbfile  = fopen (tmpp, "w");
   if (!dbfile)
     return;
index 75e5c93b1472dbc4d21f0f7fa5bb82aaae849f1f..17dccf862aba6bacf3b041806fb85b2b1a5751b9 100644 (file)
@@ -154,7 +154,7 @@ create_name (Babl *source, Babl *destination, int type)
 {
   if (babl_extender ())
     {
-      snprintf (buf, 512 - 1, "%s %i: %s%s to %s",
+      snprintf (buf, sizeof (buf), "%s %i: %s%s to %s",
                 BABL (babl_extender ())->instance.name,
                 collisions,
                 type == BABL_CONVERSION_LINEAR ? "" :
@@ -162,18 +162,16 @@ create_name (Babl *source, Babl *destination, int type)
                 type == BABL_CONVERSION_PLANAR ? "planar " : "Eeeek! ",
                 source->instance.name,
                 destination->instance.name);
-      buf[511] = '\0';
     }
   else
     {
-      snprintf (buf, 512 - 1, "%s %s to %s %i",
+      snprintf (buf, sizeof (buf), "%s %s to %s %i",
                 type == BABL_CONVERSION_LINEAR ? "" :
                 type == BABL_CONVERSION_PLANE ? "plane " :
                 type == BABL_CONVERSION_PLANAR ? "planar " : "Eeeek! ",
                 source->instance.name,
                 destination->instance.name,
                 collisions);
-      buf[511] = '\0';
     }
   return buf;
 }
index 4ed14d0ef455ea88110295a5ae0b30ca2d3f0a5f..079272c753d83b331155f01fdc84810b7d4abde1 100644 (file)
@@ -135,8 +135,8 @@ format_new_from_format_with_space (const Babl *format, const Babl *space)
 {
   Babl *ret;
   char new_name[256];
-  sprintf (new_name, "%s-%s", babl_get_name ((void*)format),
-                              babl_get_name ((void*)space));
+  snprintf (new_name, sizeof (new_name), "%s-%s", babl_get_name ((void*)format),
+                                                  babl_get_name ((void*)space));
   ret = babl_db_find (babl_format_db(), new_name);
   if (ret)
     return ret;
@@ -161,6 +161,7 @@ create_name (const BablModel *model,
 {
   char            buf[512] = "";
   char           *p = &buf[0];
+  ssize_t         left;
   int             i;
   int             same_types = 1;
   const BablType**t          = type;
@@ -168,9 +169,11 @@ create_name (const BablModel *model,
   BablComponent **c1         = component;
   BablComponent **c2         = model->component;
 
-
-  sprintf (p, "%s ", model->instance.name);
+  left = 512;
+  snprintf (p, left, "%s ", model->instance.name);
   p += strlen (model->instance.name) + 1;
+  left -= strlen (model->instance.name) + 1;
+  babl_assert (left >= 0);
 
   i = components;
   while (i--)
@@ -202,7 +205,7 @@ create_name (const BablModel *model,
 
   if (same_types)
     {
-      sprintf (p, "%s", first_type->instance.name);
+      snprintf (p, left, "%s", first_type->instance.name);
       return babl_strdup (buf);
     }
 
@@ -210,11 +213,14 @@ create_name (const BablModel *model,
 
   while (i--)
     {
-      sprintf (p, "(%s as %s) ",
+      snprintf (p, left, "(%s as %s) ",
                (*component)->instance.name,
                (*type)->instance.name);
       p += strlen ((*component)->instance.name) +
            strlen ((*type)->instance.name) + strlen ("( as ) ");
+      left -= strlen ((*component)->instance.name) +
+              strlen ((*type)->instance.name) + strlen ("( as ) ");
+      babl_assert (left >= 0);
       component++;
       type++;
     }
@@ -226,7 +232,7 @@ ncomponents_create_name (const Babl *type,
                          int         components)
 {
   char buf[512];
-  sprintf (buf, "%s[%i] ", type->instance.name, components);
+  snprintf (buf, sizeof (buf), "%s[%i] ", type->instance.name, components);
   return babl_strdup (buf);
 }
 
index 45ba8fa4f1b7bd3bc4b1a03a485adf0f70b3bd7f..20841b9483125cc8da8c5f62c25124ab80cf5c0d 100644 (file)
@@ -973,7 +973,7 @@ char *babl_icc_get_key (const char *icc_data,
   {
     char tag[5];
     int val = icc_read (u32, 64);
-    sprintf (tag, "%i", val);
+    snprintf (tag, sizeof (tag), "%i", val);
     return strdup (tag);
   } else if (!strcmp (key, "tags"))
   {
index 0589e3f383badcadeb57fc70b9949568703687dc..e0b4f7a687fc8b9c4f0cd380a6f99fd577b37844 100644 (file)
@@ -65,7 +65,7 @@ babl_backtrack (void)
 {
   char buf[512];
 
-  sprintf (buf, "echo bt>/tmp/babl.gdb;"
+  snprintf (buf, sizeof (buf), "echo bt>/tmp/babl.gdb;"
            "gdb -q --batch -x /tmp/babl.gdb --pid=%i | grep 'in ''babl_die' -A40", getpid ());
   return system (buf);
 }
index d9e4d2fd692e3559682824db9235d986aac5e361..d0229e4d4aba6ac73693e845da0db5539f7e1dc7 100644 (file)
@@ -71,7 +71,7 @@ mem_stats (void)
 {
   static char buf[128];
 
-  sprintf (buf, "mallocs:%i callocs:%i strdups:%i dups:%i allocs:%i frees:%i reallocs:%i\t|",
+  snprintf (buf, sizeof (buf), "mallocs:%i callocs:%i strdups:%i dups:%i allocs:%i frees:%i reallocs:%i\t|",
            mallocs, callocs, strdups, dups, mallocs + callocs + strdups + dups, frees, reallocs);
   return buf;
 }
index 823ff0c3421c6ae68d41404188786976f067b544..fda96ae1f638e21799db861810e6f4f5bd788164 100644 (file)
@@ -483,7 +483,7 @@ const Babl *babl_new_palette (const char  *name,
   if (!name)
     {
       static int cnt = 0;
-      sprintf (cname, "_babl-int-%i", cnt++);
+      snprintf (cname, sizeof (cname), "_babl-int-%i", cnt++);
       name = cname;
     }
   else
index adc9e7a92c5a8612dc977699784b779dfe743ad4..781e41997461e17875320ab45953007a7dbc3ee0 100644 (file)
@@ -285,9 +285,9 @@ babl_space_from_rgbxyz_matrix (const char *name,
   space_db[i]=space;
   space_db[i].instance.name = space_db[i].name;
   if (name)
-    sprintf (space_db[i].name, "%s", name);
+    snprintf (space_db[i].name, sizeof (space_db[i].name), "%s", name);
   else
-    sprintf (space_db[i].name, "space-%.4f,%.4f_%.4f,%.4f_%.4f_%.4f,%.4f_%.4f,%.4f_%s,%s,%s",
+    snprintf (space_db[i].name, sizeof (space_db[i].name), "space-%.4f,%.4f_%.4f,%.4f_%.4f_%.4f,%.4f_%.4f,%.4f_%s,%s,%s",
                        rx, gx, bx,
                        ry, gy, by,
                        rz, gz, bz,
@@ -348,10 +348,11 @@ babl_space_from_chromaticities (const char *name,
   space_db[i]=space;
   space_db[i].instance.name = space_db[i].name;
   if (name)
-    sprintf (space_db[i].name, "%s", name);
+    snprintf (space_db[i].name, sizeof (space_db[i].name), "%s", name);
   else
           /* XXX: this can get longer than 256bytes ! */
-    sprintf (space_db[i].name, "space-%.4f,%.4f_%.4f,%.4f_%.4f,%.4f_%.4f,%.4f_%s,%s,%s",
+    snprintf (space_db[i].name, sizeof (space_db[i].name),
+             "space-%.4f,%.4f_%.4f,%.4f_%.4f,%.4f_%.4f,%.4f_%s,%s,%s",
              wx,wy,rx,ry,bx,by,gx,gy,babl_get_name (space.trc[0]),
              babl_get_name(space.trc[1]), babl_get_name(space.trc[2]));
 
index 7524ef8c10961b9beda8ac293cb3b11c4dbf8da1..0a007109c9b2a64daa33deefb9b7740ff07a892a 100644 (file)
@@ -292,11 +292,11 @@ babl_trc_new (const char *name,
   trc_db[i]=trc;
   trc_db[i].instance.name = trc_db[i].name;
   if (name)
-    sprintf (trc_db[i].name, "%s", name);
+    snprintf (trc_db[i].name, sizeof (trc_db[i].name), "%s", name);
   else if (n_lut)
-    sprintf (trc_db[i].name, "lut-trc");
+    snprintf (trc_db[i].name, sizeof (trc_db[i].name), "lut-trc");
   else
-    sprintf (trc_db[i].name, "trc-%i-%f", type, gamma);
+    snprintf (trc_db[i].name, sizeof (trc_db[i].name), "trc-%i-%f", type, gamma);
 
   if (n_lut)
   {
@@ -430,7 +430,7 @@ babl_trc_formula_srgb (double g, double a, double b, double c, double d)
       fabs (c - (-3417))  < 0.01)
     return babl_trc ("sRGB");
 
-  sprintf (name, "%.6f %.6f %.4f %.4f %.4f", g, a, b, c, d);
+  snprintf (name, sizeof (name), "%.6f %.6f %.4f %.4f %.4f", g, a, b, c, d);
   for (i = 0; name[i]; i++)
     if (name[i] == ',') name[i] = '.';
   while (name[strlen(name)-1]=='0')
@@ -446,7 +446,7 @@ babl_trc_gamma (double gamma)
   if (fabs (gamma - 1.0) < 0.01)
      return babl_trc_new ("linear", BABL_TRC_LINEAR, 1.0, 0, NULL);
 
-  sprintf (name, "%.6f", gamma);
+  snprintf (name, sizeof (name), "%.6f", gamma);
   for (i = 0; name[i]; i++)
     if (name[i] == ',') name[i] = '.';
   while (name[strlen(name)-1]=='0')